Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple ogre + OSX #5

Merged
merged 19 commits into from
Oct 8, 2021
Merged

Conversation

Tobias-Fischer
Copy link
Contributor

@Tobias-Fischer Tobias-Fischer commented Mar 23, 2021

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@Tobias-Fischer
Copy link
Contributor Author

@Tobias-Fischer
Copy link
Contributor Author

@conda-forge-admin, please rerender

conda-forge-linter and others added 2 commits March 23, 2021 22:10
@Tobias-Fischer
Copy link
Contributor Author

@conda-forge-admin, please rerender

@github-actions
Copy link
Contributor

Hi! This is the friendly automated conda-forge-webservice.
I tried to rerender for you, but it looks like there was nothing to do.

@Tobias-Fischer
Copy link
Contributor Author

@conda-forge-admin, please rerender

@Tobias-Fischer
Copy link
Contributor Author

Hey @traversaro - note that the conda_build_config had the wrong suffix (yml instead of yaml) and thus wasn't picked up (i.e. not the macos stuff either). Could be relevant for the osx builds.

@traversaro
Copy link
Contributor

Hey @traversaro - note that the conda_build_config had the wrong suffix (yml instead of yaml) and thus wasn't picked up (i.e. not the macos stuff either). Could be relevant for the osx builds.

Yes, I noticed and fixed as well in the other PRs, sorry for not highlighting it.

@Tobias-Fischer Tobias-Fischer changed the title Multiple ogre Multiple ogre + OSX Mar 23, 2021
@Tobias-Fischer
Copy link
Contributor Author

On ogre 1.10 sth pulls in libarchive - I tried adding it to the host deps but still the system one is found:

21/32 Test #21: INTEGRATION_thermal_camera_plugin ...........Subprocess aborted***Exception:   0.25 sec
dyld: Symbol not found: _iconv
  Referenced from: /usr/lib/libarchive.2.dylib
  Expected in: $PREFIX/lib/libiconv.2.dylib
 in /usr/lib/libarchive.2.dylib

I think they are exactly the same tests that fail for ogre 1.12, there with

dyld: Library not loaded: /System/Library/Frameworks/Accelerate.framework/Versions/A/Frameworks/vecLib.framework/Versions/A/libBLAS.dylib
  Referenced from: /System/Library/Frameworks/Accelerate.framework/Versions/A/Frameworks/vecLib.framework/Versions/A/vecLib
  Reason: Incompatible library version: vecLib requires version 1.0.0 or later, but libBLAS.dylib provides version 0.0.0

@traversaro
Copy link
Contributor

Interesting, so both ogre 1.10 and 1.12 do not work correctly when running the tests during conda-build. On the other hand, as far I understand (for 1.12 from #3 (comment), for 1.10 because people actually use Gazebo on macOS) it seems that on most users setup ogre for some reason works. I wonder if this means that we should skip the test on macOS for now, open an issue to track this but for the time being enabling the macOS builds for both ogre 1.10 and 1.12 .

@wolfv
Copy link
Member

wolfv commented Mar 24, 2021

Hmm. When I tried locally, I could execute the unit test fine when changing the directory to be in the same directory as the exectuable.

E.g.

cd test; ./UNIT_Lidar_test

was running fine, while ctest ... had the same issues as in the tests above.

A wild guess in the dark could be that this might be fixed by setting some rpath related cmake variables. :)

@Tobias-Fischer
Copy link
Contributor Author

@wolfv - was that the case for both the ogre1.10 and ogre1.12 builds?

@wolfv
Copy link
Member

wolfv commented Mar 24, 2021

I only tested 1.12

@Tobias-Fischer
Copy link
Contributor Author

Alright, that's even better as the test error there seems "more severe". @traversaro would you be happy to look into the rpath stuff? You're probably ten times faster than what I'd be.

@traversaro
Copy link
Contributor

I'll try. I just realized that I can actually get a test macOS machine at work, I will try to use it for this debug.

@Tobias-Fischer
Copy link
Contributor Author

So setting DYLD_LIBRARY_PATH or DYLD_FALLBACK_LIBRARY_PATH as suggested by @wolfv on gitter didn't fix the tests .. :(

@Tobias-Fischer
Copy link
Contributor Author

Two options I guess: Disabling the failing tests for now and open an issue, or digging deeper (including RPATH etc). What do you think @traversaro @wolfv?

@Tobias-Fischer
Copy link
Contributor Author

Quick reminder @traversaro @wolfv

@Tobias-Fischer Tobias-Fischer mentioned this pull request Mar 30, 2021
5 tasks
@traversaro
Copy link
Contributor

Quick reminder @traversaro @wolfv

I will get access to a real mac today. I will check if a basic example is able to start, and if yes I would merge, otherwise I would wait.

@Tobias-Fischer
Copy link
Contributor Author

Hi @traversaro - did you have a chance to check this on a physical mac?

@traversaro
Copy link
Contributor

Sorry, still on the backlog, but on the top of it!

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I was trying to look for recipes to lint for you, but it appears we have a merge conflict.
Please try to merge or rebase with the base branch to resolve this conflict.

Please ping the 'conda-forge/core' team (using the @ notation in a comment) if you believe this is a bug.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@Tobias-Fischer
Copy link
Contributor Author

@conda-forge-admin, please rerender

conda-forge-linter and others added 2 commits May 13, 2021 02:19
@traversaro
Copy link
Contributor

I am finally back on it. I don't think anything that used ignition-rendering-ogre on macOS was working back in time due to gazebosim/gz-rendering#454 . However, I think we may probably have the same tests problem we have in conda-forge/libignition-rendering4-feedstock#19 (comment) . So, I think we can revive this PR, supress any test failure we may have, and then check if the tests of the fortress version work fine (we can also fix older version if there is interest, but I would start by getting the Fortress version to work). What do you think @Tobias-Fischer ?

@traversaro
Copy link
Contributor

@conda-forge-admin, please rerender

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I was trying to look for recipes to lint for you, but it appears we have a merge conflict.
Please try to merge or rebase with the base branch to resolve this conflict.

Please ping the 'conda-forge/core' team (using the @ notation in a comment) if you believe this is a bug.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@traversaro
Copy link
Contributor

@conda-forge-admin, please rerender

@traversaro
Copy link
Contributor

Actually, we can also continue to skip osx for now, and then enable as part or after #16 . The important thing that I prefer is to merge this one before #16 .

@Tobias-Fischer
Copy link
Contributor Author

I'd be happy for this one to be merged in.

recipe/meta.yaml Outdated
run:
- __osx >={{ MACOSX_DEPLOYMENT_TARGET|default("10.9") }} # [osx and x86_64]
- {{ pin_compatible('ogre', max_pin='x.x') }}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be needed actually.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is true for the latest version of ogre, but note that here we link a bit old versions such as 1.10 or 1.12, the run_exports are set correctly also there?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, let's remove it then.

@traversaro traversaro merged commit c4600b5 into conda-forge:master Oct 8, 2021
@Tobias-Fischer Tobias-Fischer deleted the multiple-ogre branch October 10, 2021 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants